Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/row select #704

Closed
wants to merge 1 commit into from
Closed

Feature/row select #704

wants to merge 1 commit into from

Conversation

iKrushYou
Copy link

Implemented the option rowSelect to handle case in comment of #409

rowSelect default is false. When true, the row will cause the mouse to change to pointer to indicate that the row is selectable (an onClick will fire)

Screen Shot 2019-06-17 at 10 02 50 PM

And, when the user clicks on the row, it will highlight a different color to indicate that the row is being pressed like a button.

Screen Shot 2019-06-17 at 10 03 19 PM

The reason for this solution instead of just overriding the theme as suggested here is my usecase involves many different tables across the application -- some of which have clickable rows and some which do not. So this seemed like the best solution overall.

…with mouse as well as activates cursor: pointer
@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.102% when pulling 53010d3 on iKrushYou:row-select into dc45413 on gregnb:master.

@iKrushYou
Copy link
Author

Hey @gabrielliwerant how do I assign to someone? I think that's the procedure right?

@gabrielliwerant
Copy link
Collaborator

At the moment, I'm the only one that can really review these, and I tend to assign myself with whatever is next of my list.

To be honest, this PR would be low priority for me and I'm actually not sure I agree a change to the library is needed here. As you've already mentioned, this behavior is already customizable with the options available to you in the library. It's not really a good practice to add something to library being used by many people just because one individual has a very specific use case. I have to consider the long term implications on maintaining the library and the growing complexity of the API, and measure that against the utility being provided. For that reason, I'm reluctant to add features that are already possible through individual customization.

Another thing you can do, if it's really important to you to maintain this feature for your own code, is to use your own fork in your projects. You will need to keep it up to date yourself if you want the latest and greatest from this library as it moves forward, but otherwise, there's nothing stopping you from implementing what you need without it being included in the API of the main project.

@gabrielliwerant
Copy link
Collaborator

gabrielliwerant commented Aug 5, 2019

@iKrushYou The option to select on the row instead of the checkbox (or expand icon for expandable rows) was added to this release: https://github.com/gregnb/mui-datatables/releases/tag/2.4.0. When these options are true, the cursor will change to indicate it can be clicked. Does this help with your use case?

@patorjk
Copy link
Collaborator

patorjk commented Jun 1, 2020

What described here is mostly available in the table right now with the selectableRowsOnClick option, and will fully be available in the v3 release. What's currently missing: I noticed the pointer icon still appears on rows that aren't selectable (fixed in v3) and there isn't an option to hide the checkboxes (this will be added in v3 with a selectableRowsHideCheckboxes option). Closing for now.

@patorjk patorjk closed this Jun 1, 2020
@patorjk patorjk mentioned this pull request Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants